Skip to content

Fix renaming file#12196

Merged
chsienki merged 3 commits intodotnet:mainfrom
chsienki:fix_renaming_file
Sep 8, 2025
Merged

Fix renaming file#12196
chsienki merged 3 commits intodotnet:mainfrom
chsienki:fix_renaming_file

Conversation

@chsienki
Copy link
Member

@chsienki chsienki commented Sep 8, 2025

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2566661

We had a caching bug where we wouldn't check the name of the additional file when considering if it had changed. That means that when renaming a file in VS, we don't update the host outputs, so when we try to find the generated document, it only contains an entry for the old name, not the new one.

This bug can also be worked around by the user by typing in the document, which causes it to be detected as different and brings the state back up to date.

@chsienki chsienki requested a review from a team as a code owner September 8, 2025 19:48
@chsienki chsienki added the area-compiler Umbrella for all compiler issues label Sep 8, 2025
@chsienki
Copy link
Member Author

chsienki commented Sep 8, 2025

@dotnet/razor-tooling for reviews please.

if (other is null ||
CssScope != other.CssScope)
CssScope != other.CssScope ||
PhysicalPath != other.PhysicalPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this compare using PlatformUtilities.OSSpecificComparer? Or, is it OK if a case-insensitive rename on Windows evicts the cached item?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The casing of the filename actually flows through to the component name, which is case sensitive.

Counter.razor and CouNter.razor produce <Counter /> and <CouNter /> respectively and they won't match as valid components if the casing doesn't also match.

So it's correct that we should evict with a case sensitive rename, as its effectively now a different component.

I should add a test to cover that. Will do so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that at least helps explain why tooling and compiler differ in their approaches to case sensitivity. I suspect tooling might have a few edge case bugs around this. I might log an issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation @chsienki! That was super helpful to me.

@chsienki chsienki merged commit 94695f9 into dotnet:main Sep 8, 2025
11 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Sep 8, 2025
@akhera99 akhera99 modified the milestones: Next, 18.0 P1, 18.0 P2 Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-compiler Umbrella for all compiler issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants